Skip to content

revert: restore four lane-3 skill cuts that failed negative controls - #12247

Merged
kyle-sexton merged 5 commits into
mainfrom
revert/lane3-negative-control-fails
Nov 2, 2026
Merged

kyle-sexton merged 5 commits into
mainfrom
revert/lane3-negative-control-fails

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #6604
Refs: #6599
Refs: #6605
Refs: #6609

Summary

Restores four skill-text cuts that lane 3 merged as "measured no-loss". Retroactive negative controls (the owner's rule in rewrite-a-plugin.md, #12222) showed that the suite behind each cut scores a deliberately broken copy about the same as the shipped text. So none of these cuts was measured, and under the rule none stays. Each issue stays open with its control result posted.

Fix

One commit per part, each with a patch changelog fragment:

Verification

Every control ran on Sonnet 5.5 with judge Opus. Each broken copy was launched together with a fresh pass on main's text. The rule's PASS bar is a noise-report case drop, or a mean drop of at least 0.15.

Part Broken copy main broken Result
curate-language description one-line generic description should-trigger 51/51 51/51 FAIL: the single-run case drop did not reproduce
attribution setup body 5 procedure sections emptied 1.000 0.942 FAIL: no case drop
dometrain sync body check and Output emptied 1.000 1.000 FAIL
kindle-dedrm manage body 6 procedure sections emptied 0.898 0.889 FAIL: no case drop
kindle-dedrm manage description one-line generic description should-trigger 18/24 12/24 PASS: 3 case drops, all reproduced
kindle-dedrm setup body check and apply emptied 1.000 0.833 PASS: a 0.167 drop in all 5 runs

Related

🤖 Generated with Claude Code

kyle-sexton and others added 4 commits November 1, 2026 22:42
…cription

The shorter description shipped in #12114 on an eval that cannot tell it
from "Helper for repository maintenance tasks.": on Sonnet 5.5 both fired
51/51 should-trigger probes, and the one should-not case drop did not
reproduce in a confirmation run. Under the negative-control rule the cut
was unmeasurable, so the earlier text returns until a suite can measure it.

Refs: #6604

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t removed

A copy of the setup body with its five procedure sections emptied scored
0.942 against main's 1.000 on Sonnet 5.5 (3 cases merged, no case drop),
a 0.058 drop. Under the negative-control rule the body cut was
unmeasurable, so its two removed Gotchas return. The description change
stays: the skill has disable-model-invocation, so its description never
reaches routing.

Refs: #6599

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With its check and Output sections emptied, the sync body still scored
1.000 against main's 1.000 on sy-check-report-only (Sonnet 5.5, 5 runs
each, both VALID, no grader miss). Under the negative-control rule the
cut was unmeasurable, so the earlier body returns until a suite can
measure it.

Refs: #6605

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The manage body's negative control failed: the suite scored the shipped
body 0.898 and a copy with six procedure sections emptied 0.889, with no
case drop. The manage description and setup body keep #12121's text;
their controls passed.

Refs: #6609

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 2, 2026 03:45
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T03:46:41.490845Z c5f96a4 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c5f96a4fc7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/dometrain/skills/sync/SKILL.md Resolved
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 14f34bd83f733fd6dea16273a80d6d0b4e813f2f: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12247, incremental (c5f96a4 → 14f34bd)

No security issues found. I read the one hunk in .claude-lane/incremental.diff, which changes plugins/dometrain/skills/sync/SKILL.md.

What changed: one paragraph in the body of sync/SKILL.md is reworded:

  • Before: it explained what disable-model-invocation: true does.
  • After: it says the skill runs only when a person types /dometrain:sync. It says the frontmatter sets the flag for that reason. It links the docs, with an as-of date and a recheck trigger.

This is the fix for the settled owner thread on this file (resolution head 14f34bd, lines changed since resolution: no). I am not raising that point again.

Instruction-surface lens: the change keeps every rule. Nothing is removed or narrowed:

  • Person-only invocation: the frontmatter still enforces it, and that frontmatter is unchanged: disable-model-invocation: true at L5.
  • Report-only: check is still report-only and still the only action.
  • Maintainer-only flag: the skill still does not expose --refresh-baseline. It is documented only for a maintainer working in a clone.

Because the rules are unchanged, I did not need to consult the instruction-exception register.

Checks:

  • The changed file matches no trusted path.
  • No plugin.json or marketplace.json changes, so there are no manifest lines to report.
  • No workflow, token, trust-control, kill-switch or merge-hold changes.
  • No spend.
  • The PR context and settled-threads files contain no embedded instructions aimed at this review.
  • The prior-findings file has only last round's summary comment, which reported 0 findings. Nothing carries over.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 14f34bd83f733fd6dea16273a80d6d0b4e813f2f: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental since c5f96a4): PR #12247, no findings

Scope. This review covers one change: commit 14f34bd8 in plugins/dometrain/skills/sync/SKILL.md, which is in .claude-lane/incremental.diff. I read that hunk and the whole head file. I used .claude-lane/pr.diff only for context. The other seven files have not changed since c5f96a4, which this lane already reviewed.

What changed. In the "Consumer vs maintainer role split" paragraph, the old sentence explained what disable-model-invocation: true does ("reachable only by a human's explicit invocation, never by the model on its own initiative"). The new text states our decision instead: the skill runs only when a person types /dometrain:sync, so the frontmatter sets the flag. It also adds a links-only record for the flag:

  • a pointer to https://code.claude.com/docs/en/skills#control-who-invokes-a-skill
  • an as-of date, 2026-11-02
  • a recheck trigger, "when that section changes"

Checks

  • Path-scoped rule (.claude/rules/skill-bodies-state-current-rules.md, which covers plugins/*/skills/**): the new sentence no longer restates the flag's upstream behavior as our reason. The record has all three required parts, and the docs pointer includes a section anchor. It passes.
  • Matches the frontmatter: the head file's frontmatter still sets disable-model-invocation: true (SKILL.md#L5). The rest of the body still agrees with it, including the description and the Boundaries "Never model-invocable" line.
  • Settled thread: the one settled thread on this file (owner reply "VALID (fix now)", resolved at this head) is the fix this commit makes. I am not raising it again.
  • Not a finding: the PR body says git diff <cut>^ HEAD is empty for each restored file, apart from the two kept descriptions. That claim no longer holds for dometrain sync, since this commit moves it away from the pre-feat(dometrain): concise sync body (round 2, measured no-loss) #12162 text on purpose. The fragment's "back to its earlier text" wording (revert-6605-sync-body-f2a6c9d4.md) is likewise now approximate. The author may want to update the PR description; I don't count this as a finding.

Scope and inputs

  • Plugin manifests: none changed, so there are no manifest lines.
  • Trusted paths: the changed file matches no trusted-path glob.
  • Closing issues: none. The PR uses only Refs:.
  • Prior findings: none. The round cap of 3 did not apply.
  • Embedded instructions: none in the PR context, settled-threads or prior-findings files.
  • Not verified by me: I had no shell, so I ran no commands.

The restored role-split sentence presented the flag's upstream behavior
as the reason. State the decision (a person runs the check) and point at
the docs section with an as-of date and recheck trigger.

Refs: #6605

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant